Swap fetching jobs between units when both trips get shorter - #197

My disposition is similar to #193: this is an interesting concept, and the reported delivery improvements are encouraging, but I want stronger evidence that it works before committing to merge.

Please put reproducible before/after comparisons on the PR, ideally side-by-side examples showing workers changing assignments and the resulting deliveries/travel/congestion. Include representative ordinary and large games, multiple seeds, and neutral or negative results. Separate the benefit of swaps from the underlying #193 changes. If you already have more evidence privately, please share the fixtures, commands and results here.

I am especially concerned about the reported additional 13–18% reduction in large-game simulation throughput. That is a substantial cost on top of #193. Can we make the assignment search more efficient while retaining most of the benefit? The current full-team scan after every hire, plus the rolling scans, seems like the main place to investigate.

Some alternatives worth profiling rather than assuming:

• A shared, bounded comparison budget covering both hires and periodic checks, with newly hired units queued for consideration instead of each triggering an unrestricted scan. Since per-hire scans dominate your measurements, thinning only the periodic sweep does not address the main cost. If searches span ticks, revalidate both jobs and eligibility before applying a swap.
• Maintain an index/list of eligible fetchers, partitioned where useful by carried resource or job compatibility, so searches avoid empty slots and obviously ineligible partners. Compare the bookkeeping cost with the saved work.
• Evaluate a bounded shortlist of plausible partners, perhaps spatially or by job, against the exhaustive search. Spatial proximity alone is not sufficient on maps with obstacles, so measure both CPU savings and lost delivery/travel benefit.
• Verify that candidate scoring really is read-only with respect to gradient construction/refresh. jobCost calls buildingAvailable and resourceAvailable; please instrument those paths to substantiate the claim that comparisons never build a gradient.

I do not require that any particular approach above be used. I would like profiles identifying the actual cost, and a measured tradeoff between search effort, simulation throughput and gameplay benefit, against an updated baseline incorporating master's pathfinding optimizations.

Dedicated regression coverage is also needed: valid working-list/assignment bookkeeping after swaps, carried-resource compatibility, hunger/qualification checks, repeated swaps or oscillation, and deterministic behavior including save/load. The existing unchanged test suite and one repeated-seed run do not establish those properties.

One clarification for the presentation: the implementation requires the combined journey to improve; it does not require both individual trips to get shorter, as the title suggests. Please make that explicit.

Interested in pursuing this, but not approving it for merge yet.

A more concrete performance recommendation after tracing the implementation at 3613f93:

First, make speculative scoring genuinely read-only. The current jobCost precheck only verifies that b->globalGradient[swimClass] is non-null. buildingAvailable then calls buildingGradient, which can rebuild a dirty field once DIRTY_REBUILD_TICKS has elapsed. resourceAvailable calls getGradient -> getResourceGradient, which allocates and propagates a missing resource field. roundTripDistance also calls resourceAvailable. Thus a candidate comparison can trigger a whole-map calculation despite the current comment. These calls also update some gradient-use timestamps, potentially retaining fields because of speculative comparisons.

I suggest explicit cached-distance probe helpers that never allocate, refresh, or change cache-use timestamps. Preserve reachability/locked checks and the existing building-neighbor probe semantics; skip/defer a candidate when a required field is missing or invalid under the chosen freshness policy. Ordinary navigation remains responsible for constructing fields. This can change swap availability/timing, so measure it as a behavioral variant, not a byte-identical optimization. First instrument rebuilds, allocations and time attributable to scoring to establish how much this actually costs.

Then put all swap searches under one deterministic work budget. Remove the synchronous exhaustive search from the hire path. Enqueue the newly hired unit once, and share a fixed candidate-inspection budget with the periodic search. Try 8/16/32 candidate inspections per team per tick as experimental settings; these are sweep points, not a claim that one is optimal. Use simulation counters, never a wall-clock timeout, because this is a lockstep engine.

A simple initial implementation can retain a stable unit-slot cursor and bound every inspected slot, including ineligible slots. If sparse slots dominate, build a stable eligible-unit list once per team per tick (or maintain one with measured overhead), then inspect candidates from that list. Do not rebuild that list for every hire. Keep periodic work from starving behind the hire queue and coalesce duplicate queue entries.

An exhaustive search can be spread across ticks, but positions/jobs may change while it runs. Store unit identities safely, not unvalidated long-lived pointers, and recompute both current jobs, all eligibility checks and the gain immediately before a swap. A best candidate collected across ticks is not guaranteed to remain the instantaneous exhaustive optimum. A bounded batch that chooses its best valid partner is simpler; either way this deliberately trades some search quality/latency for bounded work. Queue/cursor state that influences future swaps also needs deterministic save/load handling.

For scale, today an eligible search visits 1,024 slots and evaluates up to three additional job costs per compatible teammate. The four periodic calls do not necessarily all search (empty/ineligible initiators return early), so 4,096 slot visits per team per tick is an upper bound, not the actual workload. Every eligible hire adds another full scan without a shared cap. This explains why reducing only periodic checks need not help the measured hire-dominated case.

I would benchmark incremental variants: current swaps; read-only scoring; then read-only scoring plus the shared budget. Report total simulation CPU/throughput, worst tick times, comparisons, scoring-induced gradient builds, queue delay, and deliveries/travel against both no-swaps and exhaustive-swaps baselines. A reasonable proposed acceptance target is around <=1–2% total simulation overhead on representative large games while retaining most of the delivery benefit, with uncertainty reported. That is a target to demonstrate, not a speedup established by this analysis.

My preference is to start with these two changes before introducing a spatial index: they directly address unbounded work and unintended full-map computations, and avoid assuming that nearby workers are necessarily the best partners on obstacle-heavy maps.

Simple example:
Play FourSquare1 and try to build two bases on the two massive corn fields. If you play on master, you either have to first build one base then the other or you have to use forbidden areas to split the island in half.

For this example I put a swarm to each corn field and built an inn next to it when the swarm was ready and a swarm next to it when that was ready. The master version is fizzeling out at this step. This branch's version is thriving.

Yes, you can reduce workers and make sure you have unemployement in both bases so assignments get filled from near by units but that's micro management and a pain.

Rebased onto the reduced #193 (round trip only, on current master); the three swap commits applied with one trivial SConscript conflict. 172 unit tests, all harnesses and the resource-fetch-target regression pass. Next here, per the review above: read-only cached-distance probes for scoring (no allocation, refresh or timestamp updates during a comparison), one deterministic per-team inspection budget shared by hires and the periodic sweep, the bookkeeping/oscillation/save-load tests, and an evidence package against the current master baseline. Not asking for review until those are in.

Petri benchmark delta of this PR over its base #244 (full tables and setup in #244 (comment)): 20 seeds, 15000 ticks, four AINone colonies on Leo's petri map.

• Team 3 (8 workers, 215 job slots): deliveries 213 → 255 (+20%), tiles per delivery 21.2 → 17.1, 115 job swaps per run. All 8 workers trained after 22574 → 18476 ticks (15.0 → 12.3 game minutes, 90000-tick run, 10 seeds).
• Team 0 (108 workers, 4 inns): survivors 79.2 → 81.8, starvation deaths 28.9 → 26.2, entirely from the inn swap at booking time (61 inn swaps per run).
• Team 2 (108 workers, 2 inns): 2 fewer deaths (18 inn swaps). Team 1 papyrus: 25 tiles cleared after 3482 → 3290 ticks.

Petri2 (team 2's inns moved further apart, 80 game minutes, 10 seeds; chart in the #244 comment): team 2 keeps 26.9 workers alive against 22.4 on #244 and 22.2 on master, the whole difference being the inn swap at booking time; team 0 76.3 against 73.8 / 65.8. Deliveries over the 80 minutes: team 0 2150 vs 2057, team 2 783 vs 673, team 3 536 vs 528. On the school island all 8 workers are trained by minute 12 (16 to 17 on #244 and master); the colony goes to school as a whole between minute 11 and 13 and deliveries pause for those two minutes.

@genixpro please reconsider. I now provide solid data with AINone. The AINicowar games distorted all experiments.

Following up on the earlier review concern about jobCost's "read-only" scoring not actually being read-only: I traced it in the current pushed code and it's still open, then built and verified a fix in an isolated worktree (your branch untouched).

The problem, confirmed in the current code

jobCost's only guard checks b->globalGradient[swimClass] == NULL — that's the building gradient. It then calls Map::resourceAvailable() with no equivalent guard, and resourceAvailable → getGradient → getResourceGradient allocates a full map-sized array and runs a whole-map propagation the first time that (team, resource, swim class) triple is queried. roundTripDistance() — the fast path jobCost tries first — isn't exempt either, since it unconditionally calls resourceAvailable() internally. Separately, buildingAvailable() calls buildingGradient(), which rebuilds a dirty field once DIRTY_REBUILD_TICKS has elapsed — the null-check guard doesn't catch that case since the pointer is non-null, just stale.

Net effect: a comparison scan meant only to pick the best of several swap candidates (Team::swapTask/swapInn, up to Unit::MAX_COUNT candidates per call) can trigger a full-map gradient build purely from being looked at — even for a single fetcher with nobody to swap with, since swapTask's own outer jobCost(unit, a, r, &own) call hits the same path.

The fix

Added three genuinely side-effect-free probes that read whatever gradient already exists and return false otherwise — never allocating, never rebuilding a dirty field, never touching a cache-use timestamp:

• Map::peekBuildingDistance (counterpart to buildingAvailable)
• Map::peekResourceDistance, two overloads (counterpart to resourceAvailable)
• Map::peekRoundTripDistance (counterpart to roundTripDistance)

Swapped jobCost/innCost in TeamStep.cpp over to these. Real navigation code (hiring, actual walking) is untouched and keeps building/rebuilding fields exactly as before — only the candidate-comparison scan changed. A candidate whose field isn't built yet is simply skipped for that scan, same as today, just without the side effect.

Verification

Followed the repo's "confirm the regression actually fails against the unpatched code" convention:

• Added test/TaskSwapScoringHarness.cpp (registered in src/SConscript + CI): one scenario proves a lone fetcher being scored for a swap must not spring a whole-map resource gradient into existence; a second proves a genuine beneficial swap still executes correctly with the new probes.
• Reverted just the TeamStep.cpp half of this patch, rebuilt: the new test fails exactly where expected. Restored it, rebuilt: passes.
• Re-ran the existing suite against the full fix, all green on macOS/arm64/clang: InnSwapHarness, BuildingExpelHarness, ResourceFetchTargetHarness, RoundTripHungerGateHarness, HiringBucketHarness, FetchApportionmentHarness, and all 187 CppUnit tests.

Doesn't touch the separate throughput-cost or Petri-benchmark questions from earlier — just this allocation-during-scoring issue. Full patch below if you want to pull it in directly (applies cleanly on top of the current branch tip, 1470aa4c6):

diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml
index 52f17234a..8995ca3d3 100644
--- a/.github/workflows/build.yml
+++ b/.github/workflows/build.yml
@@ -173,6 +173,11 @@ jobs:
           scons -j$(nproc) release=1 server=0 inn-swap-test
           timeout 120s ./build/src/InnSwapHarness
 
+      - name: Build and run the task swap scoring regression
+        run: |
+          scons -j$(nproc) release=1 server=0 task-swap-scoring-test
+          timeout 120s ./build/src/TaskSwapScoringHarness
+
       - name: Test savegame loading and atomic autosaves
         run: |
           scons -j$(nproc) release=1 server=0 savegame-safety-test buffered-file-test
diff --git a/src/SConscript b/src/SConscript
index b1f3e4d4e..3dc23084d 100644
--- a/src/SConscript
+++ b/src/SConscript
@@ -670,6 +670,13 @@ if not env['server']:
     regression_test = local.Program('InnSwapHarness', regression_sources)
     local.Alias('inn-swap-test', regression_test)
 
+# Real engine regression for task-swap candidate scoring, built only by its explicit target.
+if not env['server']:
+    regression_sources = [source for source in source_files if source != 'Glob2.cpp']
+    regression_sources += local.Object('TaskSwapScoringHarness.o', '#test/TaskSwapScoringHarness.cpp')
+    regression_test = local.Program('TaskSwapScoringHarness', regression_sources)
+    local.Alias('task-swap-scoring-test', regression_test)
+
 # Real engine regression for the resource hunger gate, built only by its explicit target.
 if not env['server']:
     regression_sources = [source for source in source_files if source != 'Glob2.cpp']
diff --git a/src/map/Map.h b/src/map/Map.h
index 8bc531e1f..8e9543124 100644
--- a/src/map/Map.h
+++ b/src/map/Map.h
@@ -613,6 +613,11 @@ public:
 	bool resourceAvailable(int teamNumber, int resourceType, int swimClass, int x, int y);
 	bool resourceAvailable(int teamNumber, int resourceType, int swimClass, int x, int y, int *dist);
 	bool resourceAvailableUpdate(int teamNumber, int resourceType, int swimClass, int x, int y, Sint32 *targetX, Sint32 *targetY, int *dist);
+	//! Read-only probe of a resource gradient already built for this team, resource and swim
+	//! class: never allocates or propagates one. False, not "unreachable", when there is none
+	//! yet. For candidate scoring (comparing several units or jobs), never the real walk.
+	bool peekResourceDistance(int teamNumber, int resourceType, int swimClass, int x, int y) const;
+	bool peekResourceDistance(int teamNumber, int resourceType, int swimClass, int x, int y, int *dist) const;
 	
 	//! Follow the gradient uphill from (x, y). Returns whether a goal cell was reached; the
 	//! last position is in (targetX, targetY). Works on the Uint16 pathfinding gradients and
@@ -668,9 +673,17 @@ public:
 	//! building, read from a round-trip gradient a fetcher's walk has already built. False
 	//! when there is none or no such trip; the caller then scores by the plain distances.
 	bool roundTripDistance(Building *building, int resourceType, int swimClass, int x, int y, int *dist);
+	//! Read-only counterpart of roundTripDistance for candidate scoring: never allocates the
+	//! plain resource gradient roundTripDistance falls back on to check the resource is still
+	//! there. False, not "unreachable", whenever that gradient does not already exist.
+	bool peekRoundTripDistance(const Building *building, int resourceType, int swimClass, int x, int y, int *dist) const;
 	//! The building's gradient for a swim class, built or refreshed as needed; NULL if the building is unreachable.
 	const Uint16 *buildingGradient(Building *building, int swimClass);
 	bool buildingAvailable(Building *building, int swimClass, int x, int y, int *dist);
+	//! Read-only counterpart of buildingAvailable for candidate scoring: reads whatever
+	//! gradient already exists, without allocating one or rebuilding a dirty one. False, not
+	//! "unreachable", whenever no field exists yet.
+	bool peekBuildingDistance(const Building *building, int swimClass, int x, int y, int *dist) const;
 	//!requests the next step (dx, dy) to take to get to the building from (x,y)
 	bool pathfindBuilding(Building *building, int swimClass, int x, int y, int *dx, int *dy);
 	
diff --git a/src/map/MapResources.cpp b/src/map/MapResources.cpp
index ca552b150..93a6cdab4 100644
--- a/src/map/MapResources.cpp
+++ b/src/map/MapResources.cpp
@@ -179,6 +179,29 @@ bool Map::resourceAvailable(int teamNumber, int resourceType, int swimClass, int
 		return false;
 }
 
+bool Map::peekResourceDistance(int teamNumber, int resourceType, int swimClass, int x, int y) const
+{
+	// getGradient/getResourceGradient allocates and propagates a whole-map field the
+	// first time a (team, resource, swim class) triple is asked for; a candidate
+	// comparison must not cause that, so read the array directly instead.
+	const Uint16 *gradient = resourcesGradient[teamNumber][resourceType][swimClass];
+	if (gradient == NULL)
+		return false;
+	return gradient[coordToIndex(x, y)] > GRADIENT_UNREACHABLE;
+}
+
+bool Map::peekResourceDistance(int teamNumber, int resourceType, int swimClass, int x, int y, int *dist) const
+{
+	const Uint16 *gradient = resourcesGradient[teamNumber][resourceType][swimClass];
+	if (gradient == NULL)
+		return false;
+	Uint16 g = gradient[coordToIndex(x, y)];
+	if (g <= GRADIENT_UNREACHABLE)
+		return false;
+	*dist = gradientTiles(g);
+	return true;
+}
+
 bool Map::resourceAvailableUpdate(int teamNumber, int resourceType, int swimClass, int x, int y, Sint32 *targetX, Sint32 *targetY, int *dist)
 {
 	// distance and availability
diff --git a/src/map/pathfind/MapPathfindBuilding.cpp b/src/map/pathfind/MapPathfindBuilding.cpp
index 21996c477..bce718d3b 100644
--- a/src/map/pathfind/MapPathfindBuilding.cpp
+++ b/src/map/pathfind/MapPathfindBuilding.cpp
@@ -11,8 +11,9 @@
 
 
 
-// Building pathfinding (buildingGradient, buildingAvailable, roundTripGradient,
-// roundTripDistance, pathfindBuilding, dirtyBuildingGradients)
+// Building pathfinding (buildingGradient, buildingAvailable, peekBuildingDistance,
+// roundTripGradient, roundTripDistance, peekRoundTripDistance, pathfindBuilding,
+// dirtyBuildingGradients)
 
 namespace {
 
@@ -73,6 +74,22 @@ bool Map::buildingAvailable(Building *building, int swimClass, int x, int y, int
 	return true;
 }
 
+bool Map::peekBuildingDistance(const Building *building, int swimClass, int x, int y, int *dist) const
+{
+	// Read whatever buildingGradient last published, without its allocate-if-missing
+	// or rebuild-if-dirty side effects: a candidate comparison must not cause either.
+	const Uint16 *gradient=building->globalGradient[swimClass];
+	if (gradient==NULL || building->locked[swimClass>0])
+		return false;
+	Uint16 g=gradient[coordToIndex(x, y)];
+	for (int d=0; d<8 && g<=GRADIENT_UNREACHABLE; d++)
+		g=gradient[coordToIndex(x+tabClose[d][0], y+tabClose[d][1])];
+	if (g<=GRADIENT_UNREACHABLE)
+		return false;
+	*dist=gradientTiles(g);
+	return true;
+}
+
 
 const Uint16 *Map::roundTripGradient(Building *building, int resourceType, int swimClass)
 {
@@ -108,6 +125,24 @@ bool Map::roundTripDistance(Building *building, int resourceType, int swimClass,
 	return true;
 }
 
+bool Map::peekRoundTripDistance(const Building *building, int resourceType, int swimClass, int x, int y, int *dist) const
+{
+	const Uint16 *gradient=building->roundTripGradient[resourceType][swimClass];
+	if (gradient==NULL)
+		return false;
+	// Same staleness tolerance as roundTripDistance, but through the read-only
+	// resource check: a candidate comparison must not allocate that gradient either,
+	// and must not extend how long this round-trip field is kept alive by being
+	// merely looked at rather than actually walked.
+	if (!peekResourceDistance(building->owner->teamNumber, resourceType, swimClass, x, y))
+		return false;
+	Uint16 g=gradient[coordToIndex(x, y)];
+	if (g<=GRADIENT_UNREACHABLE)
+		return false;
+	*dist=gradientTiles(g);
+	return true;
+}
+
 
 bool Map::pathfindBuilding(Building *building, int swimClass, int x, int y, int *dx, int *dy)
 {
diff --git a/src/team/TeamStep.cpp b/src/team/TeamStep.cpp
index 2f5c14ecb..f19d464d3 100644
--- a/src/team/TeamStep.cpp
+++ b/src/team/TeamStep.cpp
@@ -188,8 +188,11 @@ namespace
 	}
 
 	// Tiles `u` would walk to do the job (building, resource): deliver what it
-	// carries, or fetch and carry. Only gradients that already exist are read,
-	// so a comparison never builds one. False when it cannot take the job.
+	// carries, or fetch and carry. Reads through the peek* probes, which only
+	// look at gradients that already exist: a comparison never allocates,
+	// rebuilds a dirty field, or refreshes a cache-use timestamp. False when
+	// it cannot take the job, including whenever the needed field isn't built
+	// yet — the comparison just skips that candidate this scan.
 	bool jobCost(Unit *u, Building *b, int resource, int *cost)
 	{
 		Map *map = b->owner->map;
@@ -197,13 +200,13 @@ namespace
 		if (b->globalGradient[swimClass] == NULL)
 			return false;
 		if (u->carriedResource >= 0)
-			return u->carriedResource == resource && map->buildingAvailable(b, swimClass, u->posX, u->posY, cost);
-		if (map->roundTripDistance(b, resource, swimClass, u->posX, u->posY, cost))
+			return u->carriedResource == resource && map->peekBuildingDistance(b, swimClass, u->posX, u->posY, cost);
+		if (map->peekRoundTripDistance(b, resource, swimClass, u->posX, u->posY, cost))
 			return true;
 		// No round-trip field for this class yet: the plain distances, as hiring uses them.
 		int toBuilding, toResource;
-		if (!map->buildingAvailable(b, swimClass, u->posX, u->posY, &toBuilding)
-			|| !map->resourceAvailable(b->owner->teamNumber, resource, swimClass, u->posX, u->posY, &toResource))
+		if (!map->peekBuildingDistance(b, swimClass, u->posX, u->posY, &toBuilding)
+			|| !map->peekResourceDistance(b->owner->teamNumber, resource, swimClass, u->posX, u->posY, &toResource))
 			return false;
 		*cost = toBuilding + toResource;
 		return true;
@@ -218,6 +221,7 @@ namespace
 
 	// Tiles `u` walks to reach `b`: the building's gradient, which choosing an inn
 	// already built, or the crow-flight distance findNearestFood uses for a flyer.
+	// The flightless case reads it read-only, for the same reason as jobCost.
 	bool innCost(Unit *u, Building *b, int *cost)
 	{
 		Map *map = b->owner->map;
@@ -226,7 +230,7 @@ namespace
 			*cost = 1 + (Sint32)sqrt(map->warpDistSquare(u->posX, u->posY, b->posX, b->posY));
 			return true;
 		}
-		return map->buildingAvailable(b, u->swimClass(), u->posX, u->posY, cost);
+		return map->peekBuildingDistance(b, u->swimClass(), u->posX, u->posY, cost);
 	}
 
 	// Move `u`'s booking from one inn to the other; both keep their head count.
diff --git a/test/TaskSwapScoringHarness.cpp b/test/TaskSwapScoringHarness.cpp
new file mode 100644
index 000000000..b5c5cc82a
--- /dev/null
+++ b/test/TaskSwapScoringHarness.cpp
@@ -0,0 +1,148 @@
+// SPDX-License-Identifier: GPL-3.0-or-later
+// Team::swapTask (and swapInn) score every fetching team mate as a candidate
+// to trade jobs with. A comparison used only to pick the best of several
+// options must not allocate, rebuild, or refresh anything it touches
+// (docs/development-notes.md); this harness catches the scoring path
+// allocating a resource gradient nobody has asked to walk yet, purely as a
+// side effect of jobCost() looking at a candidate.
+#include "GlobalContainer.h"
+#include "Game.h"
+#include "GameGUI.h"
+#include "Unit.h"
+#include "Team.h"
+#include "Building.h"
+#include "BuildingType.h"
+#include "IntBuildingType.h"
+#include "Race.h"
+#include "Ressource.h"
+#include <cstdio>
+#include <cstdlib>
+
+GlobalContainer* globalContainer = nullptr;
+
+static void require(bool ok, const char* message)
+{
+	if (!ok) { std::fprintf(stderr, "FAIL: %s\n", message); std::exit(1); }
+}
+
+struct World
+{
+	GameGUI gui;
+	Game& game = gui.game;
+	Team* team = nullptr;
+
+	World()
+	{
+		game.map.setSize(6, 6, GRASS);  // 64x64
+		game.map.setGame(&game);
+		game.addTeam(0);
+		team = game.teams[0];
+	}
+
+	Building* addInn(int x, int y)
+	{
+		const int typeNum = globalContainer->buildingsTypes.getTypeNum("inn", 0, false);
+		require(typeNum >= 0, "inn type exists");
+		Building* b = game.addBuilding(x, y, typeNum, 0);
+		require(b != nullptr, "inn placed");
+		game.map.setBuilding(x, y, b->type->width, b->type->height, b->gid);
+		return b;
+	}
+
+	// A worker already mid-fetch for `b`'s `resource`, as swapTask/swapInn find
+	// it: hand-set rather than hired, so the scenario controls exactly which
+	// gradients exist before the scoring call under test.
+	Unit* addFetcher(int x, int y, Building* b, int resource)
+	{
+		Unit* u = game.addUnit(x, y, 0, WORKER, 0, 0, 0, 0);
+		require(u != nullptr, "worker placed");
+		u->activity = Unit::ACT_FILLING;
+		u->displacement = Unit::DIS_GOING_TO_RESOURCE;
+		u->destinationPurpose = resource;
+		u->attachedBuilding = b;
+		u->ownExchangeBuilding = nullptr;
+		u->medical = Unit::MED_FREE;
+		u->carriedResource = -1;
+		b->unitsWorking.push_back(u);
+		return u;
+	}
+};
+
+// A lone fetcher, with nobody to swap with, still has its own job scored
+// once at the top of swapTask (jobCost(unit, a, r, &own)). That candidate
+// scoring must not be what causes STONE's gradient for this team and swim
+// class to spring into existence.
+static void scoringALoneFetcherBuildsNoResourceGradient()
+{
+	World world;
+	Building* b = world.addInn(4, 8);
+	// Give the building its own gradient the ordinary way: a unit actually
+	// walking to it, exactly as would have happened before this fetcher was
+	// ever considered for a swap. This is not part of what is under test.
+	int dist;
+	require(world.game.map.buildingAvailable(b, 0, 20, 9, &dist), "the building is reachable");
+	// A real, reachable STONE tile: if scoring were to build the gradient (the
+	// bug), peekResourceDistance would then read true here. Without a tile to
+	// find, a built-but-empty gradient reads exactly like no gradient at all,
+	// and the precondition check below could not tell the two apart.
+	require(world.game.map.incResource(20, 12, STONE, 0), "seed a reachable stone tile");
+
+	Unit* fetcher = world.addFetcher(20, 9, b, STONE);
+	require(world.game.integrity(), "scenario setup is consistent");
+
+	require(!world.game.map.peekResourceDistance(0, STONE, 0, fetcher->posX, fetcher->posY),
+		"precondition: STONE has no gradient for this team yet");
+
+	world.team->swapTask(fetcher);
+
+	require(!world.game.map.peekResourceDistance(0, STONE, 0, fetcher->posX, fetcher->posY),
+		"scoring a fetcher must not allocate a resource gradient nobody asked to walk");
+	std::puts("PASS scoring a lone fetcher builds no resource gradient");
+}
+
+// Two fetchers on different, far-apart jobs that both improve by swapping:
+// the fix must still find and make the trade, not merely refuse to allocate.
+static void aGenuineSwapStillHappens()
+{
+	World world;
+	Building* near = world.addInn(4, 8);
+	Building* far = world.addInn(44, 8);
+	int dist;
+	require(world.game.map.buildingAvailable(near, 0, 4, 8, &dist), "near building reachable");
+	require(world.game.map.buildingAvailable(far, 0, 44, 8, &dist), "far building reachable");
+	// Prime STONE's resource gradient the ordinary way (as hiring would have,
+	// before either fetcher was ever assigned): both jobs fetch the same
+	// resource, so this term is common to all four jobCost() calls below and
+	// does not affect which way the swap should go. Not part of what scenario
+	// 1 is testing, so unlike it, this priming is deliberate.
+	require(world.game.map.incResource(24, 8, STONE, 0), "seed a stone tile between the two buildings");
+	require(world.game.map.resourceAvailable(0, STONE, 0, 24, 8), "stone gradient is now built");
+
+	// Alice, beside the far building, fetches for the near one; Bob, beside
+	// the near building, fetches for the far one. Swapping shortens both.
+	Unit* alice = world.addFetcher(40, 9, near, STONE);
+	Unit* bob = world.addFetcher(8, 9, far, STONE);
+	require(world.game.integrity(), "scenario setup is consistent");
+
+	world.team->swapTask(alice);
+
+	require(alice->attachedBuilding == far && bob->attachedBuilding == near,
+		"the two fetchers trade jobs for the shorter pair of trips");
+	require(world.game.integrity(), "integrity after the swap");
+	std::puts("PASS a genuine swap between two mismatched fetchers still happens");
+}
+
+int main()
+{
+	GlobalContainer globals;
+	globalContainer = &globals;
+	globals.runNoX = true;
+	globals.settings.rememberUnit = false;
+	globals.buildingsTypes.init();
+	IntBuildingType::init();
+	Race::loadDefault();
+	scoringALoneFetcherBuildsNoResourceGradient();
+	aGenuineSwapStillHappens();
+	std::puts("Task swap scoring regressions passed");
+	return 0;
+}

Followed up on the throughput question from earlier in this thread and the cross-platform checksum coverage that was still missing for both this PR and #244. Independent build, isolated worktrees, current branch untouched.

Throughput: incremental cost of #244 vs #197's swaps

Built four variants on the same machine (macOS/arm64, release build, alternated to control for load — see below): master, feat/pathfinding-round-trip (#244 alone), the current feat/pathfinding-swaps head (#244+#197 unpatched), and #244+#197 with the read-only-scoring patch from my earlier comment applied. Same fixed tick count per variant, existing repo fixtures (games/gd-large-4ai.game, 4 teams; games/gd-bigarena-long.game, Oazis, 11 teams), 4 alternated rounds each, wall time only after letting the machine go idle (an initial contended run swung >2x between rounds purely from other builds competing for CPU — matches the "noisy under concurrent load" caution already raised here, worth flagging for anyone benchmarking this on a busy machine).

Two things this changes about the throughput discussion:

1. The earlier-cited 13–18% cost is mostly Round-trip gradients: send workers to the resource nearest for the whole trip #244, not Swap fetching jobs between units when both trips get shorter #197's swaps. Round-trip gradients alone already account for +13.3%/+18.2%; job/inn swaps add only another ~3 points on the small map and nothing measurable on the large one (Oazis is actually very slightly faster with swaps than round-trip alone — within noise, but notably not worse). If a throughput target is being set for this PR specifically, most of the cost it would be judged against isn't coming from this PR's own code.
2. The read-only-scoring fix doesn't move throughput on these fixtures. Makes sense in hindsight: once a team's resource/building gradients exist (which normal hiring builds quickly regardless of swaps), jobCost's scoring calls hit the cheap read path either way — the bug was a correctness/side-effect problem (a comparison shouldn't allocate a whole-map field, full stop, independent of whether it happens to be expensive in practice on a given map), not the source of the measured regression. Worth having, not a performance argument for merging it.

I didn't have Leo's petri2.map to reproduce his exact Petri numbers, so this uses the repo's own committed save fixtures instead — same idea (fixed initial state, fixed tick count, alternated builds), different maps. Happy to add more fixtures/seeds if useful.

Cross-platform determinism (previously unverified for either PR)

Built master and this branch's head in a second environment — a clean Ubuntu 22.04 container (glibc + GCC 11) instead of macOS (Apple Clang) — and ran the same two save fixtures headlessly with GLOB2_CHECKSUM_SIDECAR=1, 3000 ticks each. Replay files and per-tick checksum sidecars came back byte-for-byte identical between platforms in all four combinations (master × 2 maps, this branch × 2 maps).

Caveat: both environments are arm64 (my Mac and Docker Desktop's arm64 Linux containers) — a real difference in OS/libc/compiler, but not the x86_64 ISA that GitHub's ubuntu-22.04/ubuntu-24.04 runners actually use. That gap is still open; I don't have an x86_64 box to close it from here.

Bottom line

No behavior-preservation red flags from this pass — determinism holds on every platform combination I could reach, and the throughput picture is better than the raw 13–18% number suggested once #244's baseline cost is separated from #197's marginal cost. The x86_64 cross-platform gap and Leo's original Petri fixtures are the pieces I couldn't personally close.

Followed up on "does this actually help" with a broad sweep rather than relying on my earlier throughput/determinism numbers alone (those measured cost and correctness, not benefit).

Method

48 scenarios: 8 maps spanning 2–11 teams (SmallForTwo, Mazury, FourSquares1, Holiday Island 2, Archipelago, Sand River, Playground, Oazis) × 3 AI mixes (all-Nicowar mirror, all-Warrush mirror, a 5-way Nicowar/Warrush/Castor/Econo/Numbi mix) × 2 seeds. For each scenario, generated one tick-0 save with -test-games-nox --save-game-as, then replayed the same save through three builds (master, feat/pathfinding-round-trip alone, this branch's head) via --nox <save> 20000 1 — so any divergence between builds is attributable to code, not RNG, on identical starting conditions. GLOB2_TEAM_TIMELINE=1 (already in the engine, no new instrumentation needed) gave per-team units/buildings/prestige/HP every 512 ticks for free — early (~3k ticks), mid (~10k), late (~20k) all come out of one run per scenario. 144 runs total, all completed cleanly.

Result: the benefit is real, and it's specifically the swaps

Median % change vs. master, pooled across all teams/maps/seeds:

Round-trip gradients alone show a flat zero median benefit at every checkpoint — all their measured cost (from my earlier throughput comment) buys no growth advantage by themselves in this sweep. The swap logic is what actually produces the effect, and it grows over time as the efficiency gain compounds into faster hiring/building cycles.

More interesting: swaps specifically recover cases where round-trip alone hurts. Broken out by AI/map, round-trip-alone is sometimes negative — Warrush units −10.1%, Playground map −11.1%, Oazis −5.1% — and adding swaps consistently flips these positive: Warrush → +4.7%, Playground → +20.0%, Oazis → +5.1%. Mechanistically this makes sense: round-trip's one-shot assignment can lock a unit into a locally-bad job, and swaps are exactly the correction mechanism for that.

Nicowar (economy-focused, largest sample at n≈100–105) shows the cleanest signal: #244 alone +5.3%/+4.8% (units/buildings), this branch +13.8%/+12.5%.

Survival, pooled across 252 non-local team-slots: master 214 alive/2 won/36 lost → this branch 217 alive/3 won/32 lost. Modest but consistent with the "fewer starvation deaths" claim from the Petri benchmarks.

Limits

• Prestige stayed at 0 everywhere in this sweep — these are 20k-tick economies that hadn't reached prestige-granting buildings/upgrades yet, so it's just not an informative metric at this timescale, not a bug in the dump.
• This doesn't reproduce Leo's exact Petri numbers (different maps/fixtures, no petri2.map available to me), but it's a broader, independently-run sweep pointing the same direction: real, and driven by the swap logic specifically rather than the round-trip base it sits on.

Raw per-scenario timeline data available if useful for a closer look at any specific map/AI combination.